-
Notifications
You must be signed in to change notification settings - Fork 4.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Icons: Deprecate warning
and rename to cautionFilled
#67895
Conversation
// Deprecated aliases | ||
warning: _warning, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Omitting from the Icon Library in Storybook.
@@ -1,13 +1,13 @@ | |||
const keywords: Partial< Record< keyof typeof import('../../'), string[] > > = { | |||
cancelCircleFilled: [ 'close' ], | |||
cautionFilled: [ 'alert', 'caution', 'warning' ], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Making sure it's still findable with the search query "warning".
Size Change: -173 B (-0.01%) Total Size: 1.84 MB
ℹ️ View Unchanged
|
Flaky tests detected in 968437a. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/12315195769
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works for me :)
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.
To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tests well and code LGTM 👍
I've just updated a couple of RN test snapshots that were still expecting the old icon.
Oops, I was omitting native files from my search. Fixed in 7f1d0ac. |
Follow-up to https://github.com/WordPress/gutenberg/pull/66555/files#r1880778506
What?
Deprecates the
warning
icon and renames (re-exports) it ascautionFilled
.Why?
The Icons package has a convention where we have a pair of filled and unfilled icons:
The
warning
icon is a filled icon, but didn't follow the naming convention:We now have a need for a corresponding "unfilled" icon, and thus should take this opportunity to streamline the naming.
Testing Instructions
warning
icon should appear in the Storybook Icon Library with the new namecautionFilled
.warning
, but marked as deprecated in TypeScript.There are no existing usages of the
warning
icon in the app (as far as my regex skills tells me), but maybe you can find some!Screenshots or screencast